-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Purge inactive friends and avoids #541
base: master
Are you sure you want to change the base?
Purge inactive friends and avoids #541
Conversation
@L-e-x-o-n whenever you have to time can you please put this on integration server. I would like to test. Plan is to get Kobold to purge some relationships to see if it resolves his issues: https://discord.com/channels/549281623154229250/1310908634036703302 |
593928b
to
d638fc4
Compare
number * 365.0 / 12 | ||
|
||
true -> | ||
raise("Incorrect value assigned to :purge_cutoff") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put the incorrect value in the message, when debugging it's really useful.
Also, what happen if you pass an incorrect value? Does the user see a nasty server error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error should be received by the dev. The user can only choose options so would never get an error unless a future dev added an option that didn't work. So I probably need to make a test function to make sure a future dev doesn't input an option that would break.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is not really a dev here, because this value is coming from a user. Sure, using the web page as intended will not give you weird values but anyone could submit whatever they want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How could a user submit a value other than the options I have given them? All they have access to is a dropdown. The options available are hardcoded and set by the developer.
Updated based on @geekingfrog feedback. I didn't add any test for friend purge because I am using existing function made by Teifion (I just loop through them). Added test for relationship purge and other tests. |
f66e471
to
048b79a
Compare
Add more tests
048b79a
to
1934cbb
Compare
@jauggy deployed I think the idea is nice but I would be a bit worried that I might delete some friends I care about when using this feature, especially when used with more recent timeframes... A confirmation popup would be nice, it would be even nicer if that popup could show the number and a list of users that would be purged (before clicking confirm). |
I think when you deployed I will still in the middle of changes, so purging friends doesn't seem to work on integration server. |
Added a confirm dialog for purging friends. |
9a820cf
to
ba0bcbd
Compare
ba0bcbd
to
632651b
Compare
@L-e-x-o-n whenever you have time I'd like to request this be placed on integration server again. Last time I got kobold to test and it completely wiped his avoid list. https://discord.com/channels/549281623154229250/1310908634036703302/1318145574918291536 This occured probably because there's something wrong with last_login column. It's possibly always nil. I adjusted code to make it check I have no idea why Teifion made both these columns |
Deployed again |
@L-e-x-o-n had to do a few more changes. When you have a chance please deploy again to integration. Thanks. |
Done |
The purpose of this is to help fix Kobold's error https://ptb.discord.com/channels/549281623154229250/1310908634036703302
This user has 1000 avoids/blocks/ignores. It is basically difficult for him to trim that list down and it causing issues.